Break up and simplify TransportFieldCapabilitiesAction #76958
Conversation
jtibshirani
added
:Search/Search
|
Pinging @elastic/es-search (Team:Search) |
There was a problem hiding this comment.
Not sure yet exactly where things are heading with this, but I tried to keep an open mind :)
I've left some minor comments as I feel the PR is perhaps refactoring things a bit too much (just for the sake of adapting code style, perhaps less relevant for the future planned changes).
server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java
Outdated
Show resolved
Hide resolved
| }); | ||
| } | ||
| private ShardTransportHandler(IndicesService indicesService) { | ||
| this.fetcher = new FieldCapabilitiesFetcher(indicesService); |
There was a problem hiding this comment.
Not sure why this extra abstraction is needed
There was a problem hiding this comment.
This will be very helpful when we add a new per-node execution strategy in #74648. We'll use this same logic as part of the new handler, but also need to maintain this old handler for BWC with older nodes. I'll revert this and just include the commit in the PR for #74648 so it's clear why it's necessary.
server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java
Outdated
Show resolved
Hide resolved
|
Thanks for the feedback @ywelsch, I try to have refactor PRs make sense on their own but this one ventured into "please read my mind about a future change" :) |
|
@elasticmachine run elasticsearch-ci/part-2 |
`TransportFieldCapabilitiesAction` currently holds a lot of logic. This PR breaks it up into smaller pieces and simplifies its large `doExecute` method. Simplifying the class will help before we start to make field caps optimizations. Changes: * Factor some methods out of `doExecute` to reduce its length * Streamline index block checking This backport doesn't include the change "Pull AsyncShardAction out into its own class", since it's already part of a separate class in 7.x.
TransportFieldCapabilitiesActioncurrently holds a lot of logic. This PRbreaks it up into smaller pieces and simplifies its large
doExecutemethod.Simplifying the class will help before we start to make field caps
optimizations like #74648.
Changes:
doExecuteto reduce its lengthAsyncShardActionout into its own class to simplify and better matchthe code structure in 7.x